Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added rateLimit parameter to the queue #1425

Closed
wants to merge 1 commit into from

Conversation

glynnbird
Copy link

I'm a heavy user of async.queue, but if I am using it to queue calls to an API service which implements rate limiting (say 5 API calls per second), it becomes very tricky to get the queue to consume the queue within the rate limits.

To fix this, I branched the code and added a new parameter to the queue object:

var worker = function(data, done) {
  setTimeout(done, 100);
};
var concurrency = 1;
var rateLimit = 5;
var q = async.queue(worker, concurrency, rateLimit);

The queue works as normal but only proceeds at a maximum rate of 5 items per second.

The existing tests all pass and supplying a rateLimit of null or leaving it as undefined keeps the original queue unchanged.

I've added a test for the rate-limited version.

(as this is my first commit to this project, I'm not sure if I should be including the "dist" files in the PR - please let me know if you would like these included too).

@aearly
Copy link
Collaborator

aearly commented Jun 3, 2017

Sorry, but we've discussed implementing rate limiting in Async many times ( #1314 ), and have decided against it due to the incidental complexity/gotchas involved.

You're welcome to publish a standalone rate limiting queue if you wish. We'll link it in our docs.

@aearly aearly closed this Jun 3, 2017
@glynnbird
Copy link
Author

Thanks for taking the time to reply @aearly. I understand perfectly why you wouldn't want to creeping the scope any further out any more ;)

I've published my code as a standalone as suggested here: https://www.npmjs.com/package/qrate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants